feat(models): enforce per-account upstream cloud catalogs - #232
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements per-upstream-account cloud model catalogs by syncing GetCascadeModelConfigs for each active account, exposing the union of active catalogs in pool-wide listings (/v1/models + Dashboard), and enforcing per-account catalog checks during routing.
Changes:
- Added per-account cloud catalog storage + filtering logic in
src/models.js, including a pool-union view and a compatibility opt-out viaWINDSURFAPI_IGNORE_CLOUD_FILTER=1. - Updated account lifecycle + routing gates in
src/auth.jsto sync catalogs per active account, invalidate stale catalogs on key/account changes, and enforce catalog checks before entitlements/tier overrides. - Updated Dashboard endpoints and added a dedicated test suite to ensure union listings + per-account routing behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/cloud-catalog-filter.test.js | Adds coverage for union listings, per-account routing enforcement, fail-open behavior, and lifecycle invalidation. |
| src/models.js | Introduces per-account cloud catalog filtering, updates tier/model listing behavior, and stores catalog snapshots per account. |
| src/dashboard/api.js | Ensures Dashboard model/tier-access endpoints expose the filtered union instead of the full static catalog. |
| src/auth.js | Implements per-account catalog synchronization, catalog invalidation on lifecycle changes, and per-account routing checks. |
| README.md | Documents WINDSURFAPI_IGNORE_CLOUD_FILTER and clarifies listing vs routing behavior (CN). |
| README.en.md | Documents WINDSURFAPI_IGNORE_CLOUD_FILTER and clarifies listing vs routing behavior (EN). |
| .env.example | Adds example documentation for WINDSURFAPI_IGNORE_CLOUD_FILTER. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
评审结论:方向正确,但有 2 个阻塞问题 + 1 条声明不实,暂不合并感谢这个 PR。它修的问题(#231)是真的:静态模型表被 以下结论都经过实测,不是读描述得出的。 已验证做对的部分
阻塞问题1)(blocker)干旱模式会被打成全站中断
一旦干旱模式激活(credits 低于阈值),空白名单意味着 建议:池级调用点( 2)(major)上游"部分响应"没有守卫
上游少回一批模型 == 给用户断供,而唯一的出路是 声明不实,需要你确认PR 描述写:
这条在 master 上不复现。 我在干净 master 上实跑该文件:9 tests / 9 pass / 0 fail;在你的分支上也是 0 fail。请确认是不是本地环境残留(比如 结论修掉上面第 1 条(必须)和第 2 条(强烈建议),并更正测试声明后,我会重新评审。设计本身我认可,不需要重做。 |
复审
|
dwgx
left a comment
dwgx
left a comment
There was a problem hiding this comment.
Request changes — 设计认可,还剩 2 个问题 + 1 条声明待更正
前面两条 comment 里已给出实测细节(评审、复审 b8de001),这里是正式结论,方便你对着清单收尾。
设计层面我认可,不需要重做。 "发现用并集 + 路由用账号维度"这个分层是对的,b8de001 的非数组 fail-open 也是对的,请保留。connect 路径走独立 selector 命名空间门禁、没被这个 MODELS 命名空间过滤影响 —— 这点你处理对了,本仓库历史上"修复只覆盖部分协议路由"已经犯过两次。
待改清单
- (blocker) 干旱白名单不能被账号 catalog 过滤。
src/auth.js:201的getTierModels('free')与:229的freeTierModels都是池级语义。实测某账号 catalog 只含 sonnet 时白名单变成[],而空白名单会让isModelBlockedByDrought对每个模型返回 true —— 干旱一激活即全站中断。空白名单绝不能被解释成"什么都不许用"。 - (major) 给采纳加一道合理性门槛。 目前只在响应不可解析时 fail-open;响应是合法数组但内容不全时会被当权威目录(实测 pro 账号 130 → 6)。建议:新目录相比该账号上次目录、或相比静态表显著缩小时,拒绝采纳并
log.warn。上游少回一批模型等于给用户断供,而WINDSURFAPI_IGNORE_CLOUD_FILTER=1是用户报障之后才会想到的东西。 - 更正测试声明。 描述里写
test/account-add-proxy-ordering.test.js:320那条失败"also reproduces on master"。我在干净 master 上实跑是 9 tests / 9 pass / 0 fail,你分支上也是 0 fail。请确认是否本地环境残留(.env/ 代理设置)并更正描述。
第三条单独说一句,不是针对那条测试本身:本仓库合并前会逐条核实提交者的诊断链,所以测试结论的准确性比它是否通过更重要 —— 一条不实的声明会让整份验证结果都需要重新核对。你其余的诊断我复核后都成立,这条更正掉就没有信任成本了。
改完 ping 我,我重新跑全量 + 对抗验证。
|
感谢作者答复，b8de001 是修复 copilot 自动审阅提出的小问题。 您提出的问题清单修改如下：
对抗场景复测：首次同步即返回 Sonnet-only catalog 时，一次请求后直接采纳 1 个模型；已有 130 个模型的 last-known-good 突然缩小到 Sonnet-only 时，模型数为 补充同步范围：本 PR 没有新增定时 catalog refresh。 当前同步仍由事件触发：进程启动时执行，并在账号新增、重新激活或 API key 更换等现有生命周期事件后执行；长期保持 active 且凭证不变的账号不会在运行过程中定时重新拉取上游目录。因此，无账号事件时的上游 catalog 变更检测，以及相应的调度、退避和请求频率策略，明确属于本 PR 的 out of scope，适合后续单独处理。 验证结果：本地全部自动化测试通过，包括 |
答复质量很好,但代码还没推上来先说要紧的:PR head 仍是 也就是空白名单拦一切的问题原样还在。是漏 push 了,还是推到了别的分支?推上来我就重新审。 对四条答复本身的评价方案层面我基本认可,尤其第 1、3、4 条:
第 2 条有一处内部矛盾,需要你重新想一下你的缩小检查依赖"后续独立同步轮次返回完全相同的 UID 集合才采纳"。但你在同一条答复的补充说明里写:
这两句放在一起会得到:一个长期 active、凭证不变的账号永远不会有"后续轮次"。 于是上游真的缩减了目录(比如运维方在 Devin 侧调整了 allowlist)时,新目录被 quarantine 后永久卡在旧的 last-known-good 上 —— 直到进程重启或账号事件发生。 这个方向的错误比原来那条轻(宁可多暴露模型,也不要少),所以不是 blocker。但它意味着第 2 条的保护在最常见的稳态部署下永远不会解除。两个出路:
我倾向 1,因为它诚实且不引入新机制。但请在代码注释里写明这个限制,否则下一个读者会以为 quarantine 会自动解除。 另外:分支需要 rebase你的分支基于 推上来后我会跑全量 + 对抗验证,并按台账惯例记录。 |
|
先说清楚:你的方案我认可,不用重做。 干旱那条你想的办法(先算 catalog 和 free-tier 的交集,非空就照旧限制、空了才单独 fail-open)比我原来建议的更好,保住了干旱本来的用途。Dashboard 那个 不过有个情况得先确认一下:代码好像没推上来。 是推到别的分支了,还是本地还没 push?你说的四项改动我在分支上都没看到,所以没法审。方便的话推一下,或者告诉我在哪个分支,我去看。 剩下要做的,就这几条
这三条就是全部,我不会再往上加。除非你的新代码本身带出新问题 —— 那种情况我会明确说是新代码引起的,不会当成一直都有的要求。 另外分支需要 rebase 一下:master 现在到 v3.9.4 了(这几天连着修了几个流处理的 bug)。你动的是 推上来我就跑全量测试和对抗验证,通过就合并,台账也会补上。 |
b8de001 to
cf6ece1
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The new pool-wide filtering path repeatedly rebuilds the active-account union per model key, which can add avoidable per-request overhead and should be optimized before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
src/models.js:718
- filterModelKeysByCloudCatalog() currently calls isModelAllowedByCloudCatalog() once per key. When accountId is omitted and there are active accounts, isModelAllowedByCloudCatalog() rebuilds the active-account union set on every call via applicableCloudCatalogUids(), making pool listings O(models × union) unnecessarily expensive (notably listModels(), MODEL_TIER_ACCESS getters, and dashboard endpoints).
export function filterModelKeysByCloudCatalog(
keys = Object.keys(MODELS),
env = process.env,
accountId = null,
) {
const input = Array.from(keys || []);
return input.filter((key) => isModelAllowedByCloudCatalog(key, env, accountId));
}
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
昨晚测试中发现其他问题 #234 ，b8de001 是修复 Copilot 第一次提出的问题。 PR 正文也已按最新行为和验证结果更正。 本次修复更新如下：
验证结果：
请作者基于新的 PR head 复测。 |
There was a problem hiding this comment.
🟡 Not ready to approve
listModels({ env }) introduces env injection for filtering, but still reads a related flag from process.env, which can produce inconsistent behavior when callers supply an effective environment.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
src/models.js:772
listModels()now acceptsopts.envand uses it for cloud-catalog filtering, butincludeDisabledSpecialAgentstill reads fromprocess.env. WhenlistModels({ env: effectiveEnv })is used (e.g. inhandleModels()), this makes env injection partial and can produce inconsistent behavior vs the provided env overrides.
const env = opts.env ?? process.env;
const specialAgentEnabled = opts.specialAgentEnabled ?? isSpecialAgentCatalogEnabled();
const includeDisabledSpecialAgent = opts.includeDisabledSpecialAgent
?? process.env.WINDSURFAPI_SHOW_DISABLED_SPECIAL_AGENT_MODELS === '1';
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Copilot 最新的评论不在本 PR 修改范围内，考虑为过度防御，或者后续再单独处理。 |
合并 #232(按账号隔离上游模型目录)与 #233(CI x64 runner),外加检索端点的 5 条契约修复 —— 其中 2 条是 v3.9.4 自己引入的,含一条把 PII 写进 URL。 两条教训记入交接文档: - 读过某条约束不等于会遵守它。那条 PII 缺陷里,我两小时前才引用过 caller-key.js 的 PII 注释,然后亲手违反了它。 - 注释解释的是设计意图、不是当前事实。macOS x64 连续四个版本终态是 cancelled 而非排队,我每次照 workflow 注释解释就放过了,是外部贡献者发现的。 #234 已验证成立,进未修清单。测试 3072 → 3108,全量绿。
已合并 ✅ — v3.9.5复测通过,已用 验证记录(在 head
对抗验证(找新代码激活了什么):重点查了新增的 quarantine 定时器 —— 台账: 给 A 而不是 S 的理由我说明白:方案取舍(并集发现 + 账号维度路由)是这个 PR 最值钱的部分,你在 #231 里把三种替代方案逐一论证否决,这个论证本身就是贡献;两轮修复都精准命中;Dashboard 的 干旱那条你给的办法比我建议的好 —— 我原本想的是"池级调用点直接不过滤",你先算 #234 我已验证成立,打了 感谢,首次贡献质量很高。 |
#232 的逐账号确认门有两个反向的不对称,两者都会复现 #231 自己的症状 —— 部署宣传并路由账号其实用不了的模型。 一、太宽松:空/畸形响应被立即采纳 1 个 UID 的快照要隔离两轮,0 个 UID 的**一轮都不要**,而非数组 body 直接 走 applyCloudModels([]) —— 那个调用会**删除**账号快照。实测:已采纳的 101-UID 目录(filter 106/163)在一次空响应后回到 163/163,无确认轮、无重试。 截断 / 限流 / 鉴权抖动产出的恰恰就是空 body,所以唯一没有门禁的那条路, 正是最常被走到的那条。这是"把'不知道'当成'没事'":缺答案被读成了权威 答案"此账号无过滤"。 零 UID 确实有两种读法,且需要相反处理,所以拆开:尚无已采纳快照时它仍然 意味着"本部署没有云端过滤"(既有 fail-open,保持立即生效);已有快照后它 只是最大幅度的缩水,走与其他缩水相同的确认。 二、太严格:缩水但**内容变化**的目录永久楔住 确认要求 UID 集合完全相同,所以上游每轮返回的小集合只要差一个 UID 就永远 无法自我重复,候选被无限重新隔离,而**更大的**陈旧 last-known-good 一直 权威 —— 且 auth.js 只在第一轮 arm 重试,之后没有任何东西再来纠正它 (本项目没有周期性目录刷新,只靠账号生命周期事件触发)。而这恰恰是最该 生效的场景:套餐降级或权限撤销确实会让目录缩水一半以上。实测:降级后 再跑 4 轮同步,代理仍然放行账号已经失去的模型。 修法是给隔离加上限 CLOUD_CATALOG_CONFIRM_MAX_ROUNDS(3),到顶后采纳最新 快照 —— "上游连续多轮都说它很小,只是每次不同"比"一份此后无人确认过的 旧快照"是更好的证据。同时每个被隔离的轮次都 arm 下一次复查,否则上限永远 到不了。 畸形分支刻意与隔离分支不对称:它没有可确认的东西,而 trySyncModelCatalog 会跳过有 pending timer 的账号,所以给它 arm timer 反而**延迟**恢复。保持 不 arm、留作未同步,下一次同步触发时自然重取。这条分支原本危险的地方从来 不是 cancel,而是 merge 在到达它之前就擦掉了 last-known-good。 守卫 test/cloud-catalog-degenerate-response.test.js(13 条),含四条结构 守卫:merge 路径里每个 applyCloudModels 调用都必须位于 accepted:true 的 返回中(集合求差)、畸形分支不得触碰目录状态、隔离必须有显式上限、每个 隔离轮次必须 arm 复查。突变验证三种原始形态:空+畸形擦除抓 7 条、去掉 收敛上限抓 1 条、只在首轮 arm 复查抓 2 条。 另外改写 cloud-catalog-filter.test.js 两条把缺陷固化成契约的断言: - "clears a stale catalog when mergeCloudModels receives a non-array" —— 名字和断言都在要求擦除行为,那是缺陷不是契约 - "does not keep polling when the delayed confirmation returns a different candidate" —— 不再复查正是楔住的成因;改为断言"持续复查**且**有界" 两条都保留原意图、改断言方向,并在注释里写明原先测到的现象与错误解释。
两条缺陷一个症状:/v1/models 与 chat 能跑的东西不一致。 1) 目录 latch:_connectCatalogSynced 是模块级布尔,第一个账号同步后置位且永不 清除,所以往免费池里加付费账号永远不刷新 selector 集合 —— 跨账号并集压根 取不到。改成按账号存行、写入前取并集。 单纯去 latch 会比 latch 更糟:setLiveCatalogSelectors 是 clear-then-fill, 免费账号在付费账号之后同步会把集合缩回自己那点 —— 所以并集是必需的,不是 优化。存在性(并集)与 entitlement(按账号 tier)是两个问题,分开回答。 2) 发现视图:此前只测存在性,免费池照样宣传每个付费 selector,客户端点了在 chat 拿 403。#232 在 Cascade 命名空间修过这件事,但它的过滤器在 devinConnect 时 early-return 未过滤输入(那是正确的命名空间边界),所以要在 connect 命名空间重做一遍。两个行产出器都过滤 —— 只过滤第一个的话 live-only 付费 selector 仍然漏出去。 关键陷阱:只过滤会把免费池打到 0 行,而 0 行比过度宣传更糟 —— Codex/Cline 零模型直接拒绝启动。而 swe-1-6-slow 在两个行来源里都不存在(不在冻结快照、 不在 105 个 live 行、也没有 MODELS 条目),chat 却能路由它。所以发现视图必须 合成它:第三个产出器是重建的地板,不是过滤。实测 56 宣传/0 可达 → 1 行且可达。 fail-open 镜像 chat 的豁免条件(env token 被设置即豁免,不看池大小)并补空池 一臂 —— hasConnectEntitledAccount 在空池上对每个 selector 都返回 false,少这 一臂会把还没加账号的部署剥成零行。 自查修掉两条本轮自己引入的:forgetConnectCatalogForAccount 原先用动态 import().then() 写入,既落在调用方返回之后(删号后立刻读仍看得到departed 账号的 selector),又绕过了注入的 dep seam;并把 sync 路径的动态 import 收敛 到同一处解析,避免两条路径漂移。新增 clearLiveCatalogSelectors 处理'最后一个 贡献者离开'——setLiveCatalogSelectors 按设计忽略空输入,直接传空是静默 no-op。 Refs #234, #231
改了什么 / What changed
GetCascadeModelConfigsper active account, expose the active-account union through Cascade model listings, and route only through accounts whose own catalog contains the requested model.DEVIN_CONNECTor special-agent routing, start Connect synchronization without waiting for Cascade confirmation, and preserveWINDSURFAPI_IGNORE_CLOUD_FILTER=1as a compatibility escape hatch.为什么 / Why
The static model table was exposed independently by API, Dashboard, and routing paths. Cascade deployments with a restricted upstream catalog could therefore advertise models that their accounts could not use.
The initial filtering implementation also kept one process-wide snapshot from the first active account. In a mixed account pool, that could hide usable models or apply one account's catalog to unrelated accounts.
This change treats the active pool listing as the union of its account catalogs, while keeping routing decisions isolated to the selected account.
Relates to #231.
行为 / Behavior
DEVIN_CONNECTor special-agent routing. Existing Connect discovery still starts from the shared model registry and its own selector catalog, as described under Out of scope.DEVIN_CONNECTselector-catalog synchronization.WINDSURFAPI_IGNORE_CLOUD_FILTER=1restores the complete static catalog.Out of scope
This PR does not add regular time-based catalog refresh for an unchanged active account. Normal synchronization remains event-driven: it runs at startup and after existing account lifecycle events such as account creation, reactivation, or API key replacement. The only timer added here is a single 30-second confirmation retry after an anomalously small non-empty response; it does not continue polling after the candidate is accepted or replaced. General refresh scheduling, adaptive backoff, request-rate policy, and persisted last-known-good state remain out of scope.
DEVIN_CONNECTuses a different selector namespace, but its discovery paths still start from the sharedMODELSregistry before applying the Connect selector snapshot/live catalog. This PR does not make/v1/modelsor Dashboard model views account-entitlement-aware, does not isolate that shared registry, does not move the drought gate into the Connect request path, and does not silently rewrite paid-model requests toswe-1-6-slow. With Connect enabled, the displayed model set therefore remains broader than the account-entitled selector set, and a confirmed Cascade catalog can still change that display when it contributes a previously unknown row to the shared registry. Connect-specific discovery, registry isolation, and enforcement are left for a separate follow-up.测试 / Testing
Automated verification:
masterv3.9.4 before verification.npm test: 3096/3096 passed.npm run test:release: all 233 test files completed with exit code 0.test/cloud-catalog-filter.test.jsandtest/cloud-catalog-backend-boundary.test.js.DEVIN_CONNECT/v1/modelsor Dashboard views, Connect mode does not trigger the Cascade-only drought fail-open warning, and Connect selector-catalog synchronization is not delayed.test/rate-limit.test.js: expected1632000, received1631999. The exact file passes locally 13/13 and the exact shard command passes locally; this branch does not modify that test or its rate-limit implementation. The failed shard is pending a CI rerun.npm run secret-scan: passed.git diff --check: passed.Manual smoke testing with a live restricted account confirmed the expected transition: the first below-threshold snapshot emitted a warning and preserved the fail-open view, then an exact confirmation about 30 seconds later applied the restricted listing. Two live accounts governed by the same policy produced identical sorted model-ID sets, and enabling both produced the same deduplicated pool union. Toggling Connect mode confirmed the existing broader Connect discovery view described above; its displayed count can change after Cascade confirmation because both paths still share the mutable
MODELSregistry. Entitlement-aware Connect discovery and registry isolation remain in the follow-up.Not run with heterogeneous live account catalogs: unique-model union, per-account routing, and removal of one account's unique models. These paths are covered by automated mixed-pool regression tests.
Checklist
CONTRIBUTING.md([Feature] Filter model catalogs per upstream account #231)